-
-
Notifications
You must be signed in to change notification settings - Fork 623
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[8.0][ADD][website_snippet_mass_mailing_name] Snippet + name #66
[8.0][ADD][website_snippet_mass_mailing_name] Snippet + name #66
Conversation
<input | ||
name="name" | ||
class="js_subscribe_name form-control" | ||
placeholder="your name..."/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be configurable at snippet level (to include or not to include)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand you. The only purpose of this module is to include this input. If you don't want that, don't install it, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but you must be able to select the name granularity by snippet. In some occasions, you want the snippet to serve only for logued users, in other, for all introducing the name. I have another doubt now. If you are logged, the name of the user is respected?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not right now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, so at least last point should be developed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
name = post.get("name") | ||
|
||
# Update partner's name, to make it get updated in contact list later | ||
Partner.search([("email", "=", email)], limit=1).write({"name": name}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
check if you actually got a name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAIK, it doesn't matter, as write will act as dummy in that case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, @hbrunn is right. We could be removing a name from a partner.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this line as explained later.
Now that odoo/odoo#12265 is merged this module will be dramatically simplified. |
c3de0ee
to
10b9b07
Compare
Let you have the name field in the mass mailing subscription snippet. Leave most logic to base module. Make name field autofilled if user data is available. Add tour test.
10b9b07
to
cdf0d30
Compare
Well, as said before, now the module is much simplified. The JS calls are intercepted thanks to suggestions by @hbrunn. No more dependency on mass_mailing_partner. Just install it if you want that extra functionality. Thus, it has been renamed accordingly. |
Test does not work, probably due to the framework. Leaving out the last step and hope for the best for v9.
👍 Code + runbot |
"application": False, | ||
"installable": True, | ||
"depends": [ | ||
"mass_mailing", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
website?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mass_mailing depends on website.
Tested in runbot 👍 |
👍 |
Syncing from upstream OCA/social (10.0)
Let you have the name field in the mass mailing subscription snippet.
When somebody tells his name, a partner is linked (if a match is found) or created.
Leave most logic to base module.
@Tecnativa relaunching OCA/website#201, related to OCA/crm#96.